Skip to content

fix(flex): Fix active-slice ALC and TX waveform meter routing - #5344

Merged
rfoust merged 3 commits into
aethersdr:mainfrom
jensenpat:fix/5340-alc-active-slice
Sep 6, 2026
Merged

fix(flex): Fix active-slice ALC and TX waveform meter routing#5344
rfoust merged 3 commits into
aethersdr:mainfrom
jensenpat:fix/5340-alc-active-slice

Conversation

@jensenpat

@jensenpat jensenpat commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

Addresses the ALC-metering portion of #5340. On a multi-slice FLEX radio, the software-ALC gauge could select another slice's meter, especially with the observed FLEX-8400M firmware 4.2.18 source indices 0 for A and 9 for B. This change resolves ALC, direct COMPPEAK, and TX filter taps through the active slice's observed manifest block, and converts ALC using the selected meter's declared unit.

The lifecycle corrections preserve that ownership across isolated re-announcements and unit changes, preserve first-appearance wire order in the Flex decoder, and remove old routes/samples when a meter ID is removed or repurposed. Registration uses either the contextual slice map or the context-free explicit-source fallback, preventing duplicate aliases. SLC context can span individual status messages; removal or an unrelated block invalidates it. ALC resets to its empty presentation value of -20 dBFS on startup, disconnect, TX-slice changes, and active-meter removal, including when its input unit is Percent.

The single implicit-modulator fallback remains supported. A single explicitly associated meter is not volunteered to another slice, and filter diagnostics still require associated taps. The legacy source-index arithmetic is a compatibility fallback, not a FlexLib ownership guarantee.

The FT8 decode-count concern in #5340 is separate: this PR does not alter TCI audio or demonstrate equal decode counts. The issue remains open for that concern; its discussion also links #5133.

Constitution principle honored

Principle I: FlexLib attaches only SLC meters to slices; TX source numbers are not asserted to be a slice-ID contract. The preceding-block association is identified as captured firmware behavior. Principle XI: the routing and gauge-clear failures were reproduced before repair, and the added regression checks fail when the fixes are broken deliberately. Principle VI: repair validation used no live radio and no TX.

Test plan

  • Full macOS application build at 98872c35, including upstream/main d58e2b8a (the CI base), with the local ARM64 toolchain, build-codex-arm64, and -j8. RADE remains enabled. Host/system processors and the executable are ARM64; RNNoise has no x86 sources in the build graph.

  • Six registered focused CTests pass: meter_model_test, aetherd_meter_decode_test, meter_applet_capability_test, meter_applet_voltage_state_test, health_applet_test, and phone_cw_level_meter_state_test.

  • Re-announcement regression covers ALC, COMPPEAK and all SC taps for explicit and repeated-zero source indices; selected-meter unit changes, ID reuse, context invalidation, strict explicit ownership and the implicit-modulator fallback are covered.

  • Mutation checks: restoring meter-ID sorting fails the production decoder/model test; emitting zero instead of the ALC presentation floor fails both the model test and the actual Phone/CW gauge widget test. Correct production code was restored, rebuilt and all six tests passed afterward.

  • check_engine_boundary.py --strict (known baseline warnings only), check_test_registration.py --strict, gen_touchpoint_manifest.py --check, and git diff --check pass.

  • Test-boundary inspection: no new socket binds, connects, listeners or synthetic firmware peers. The decoder test injects status strings directly into an unconnected FlexBackend; the gauge test uses the production MeterModel and PhoneCwApplet in an isolated settings profile.

  • Full-app smoke limitation: an isolated offscreen launch with TX disabled reported a QRhi renderer failure and did not expose its explicit automation socket. That process was stopped. No demo connection or live-radio convergence is claimed for the repaired head. The two-slice routing and both actual ALC gauges were verified through the focused behavioral seams above.

  • CI results are recorded on this PR's current head. The focused tests above were run directly; the filtered PR CI is not treated as the full suite.

  • Fresh current-main revalidation after CI: upstream/main advanced to 4bf9c5a6 (Icom audio fix fix(icom): preserve finite AX.25 transmit audio over RS-BA1 #5311). The clean local merge with PR head 98872c35 produced tree 669aabb595fd69430473841941e1098a634b783a. The full ARM64 application build and all six focused CTests pass on that combined tree, as do strict boundary/registration, generated manifest and whitespace checks. The overlap in FlexBackend is separate capability metadata; no repair changes or additional push were needed. The five successful PR CI checks ran with the earlier main d58e2b8a; this newer integration check was performed locally.

  • Post-merge verification: squash commit f39e7d27 is signed and verified by GitHub and was confirmed on upstream/main. A concurrent TCI IQ change, 5674c7ab (feat(tci): support four concurrent DAX IQ skimmer streams #4951), became its parent during the final merge window. Its delta leaves the ALC signal consumer and all seven files of this repair intact. The six focused targets were rebuilt and all six tests passed again on the actual merged commit, with strict boundary/registration, manifest and whitespace checks also passing. The full desktop build recorded above was on the preceding 4bf9c5a6 integration tree; this final run rebuilt the focused targets and their production dependencies. Both review threads are resolved and [bug] Slice B FT8 transmissions poorly decoded by other stations — ALC shows -20dB on Slice B vs 0dB on Slice A (possible TCI regression) #5340 remains open for its separate FT8 concern.

Checklist

  • Commits are signed; the repair and current-main merge commits have verified SSH signatures.
  • No new settings keys or persistence paths.
  • Clean-room change grounded in published FlexLib ownership rules and the author's captured manifest.
  • Existing meter smoothing and UI design are unchanged; both Phone/CW ALC mirrors have regression coverage.
  • Updated docs/architecture/flex-meter-learnings.md; no CHANGELOG edit.
  • Security advisory: not applicable to this meter-routing fix.

Original author's live-radio evidence (before lifecycle repair)

The following measurements were reported by the original author on ffa9cc77. They were not repeated during this repair and do not establish live-radio behavior of 98872c35.

The author tested a FLEX-8400M running firmware 4.2.18.41174 at 14.074 MHz DIGU into an explicitly authorized ANT2 dummy load, with Tune Power 5%, a 10 W physical ceiling, and the tuner bypassed.

TX slice Selected ALC meter Raw TX source index Selected ALC Forward power SWR
A 22 0 -6.398 dBFS 3.7 W 1.00
B 40 9 -6.406 dBFS 3.7 W 1.09

During the author's Slice B burst, A's meter stayed stale at -150 dBFS while meter 40 updated the public swAlc value. The author reported both bursts completed without an abort, followed by one Slice A, zero forward power, Tune Power restored to 10%, and MOX, Tune, model transmitting and radio transmitting all false. Radio-generated two-tone testing bypassed microphone/DAX audio and did not demonstrate dynamic speech compression.

Original implementation and hardware evidence: @jensenpat using OpenAI Codex. Lifecycle repair and regression validation: @rfoust using OpenAI Codex.

@jensenpat
jensenpat marked this pull request as ready for review August 31, 2026 01:38
@jensenpat
jensenpat requested a review from a team as a code owner August 31, 2026 01:38
@jensenpat jensenpat changed the title Fix active-slice ALC and TX waveform meter routing [meters] Fix active-slice ALC and TX waveform meter routing Aug 31, 2026
@jensenpat jensenpat changed the title [meters] Fix active-slice ALC and TX waveform meter routing fix(flex): Fix active-slice ALC and TX waveform meter routing Sep 1, 2026
ten9876
ten9876 previously requested changes Sep 1, 2026

@ten9876 ten9876 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue fit

#5340's diagnosis is correct and — unusually — was verified all the way down: the claim that FlexLib attaches meters to slices only for Source == "SLC" was independently confirmed against Meter.cs:41 and Radio.cs:6168-6175 by two analysis passes, and the 8400M num=0/9 capture demonstrates the arithmetic the old resolver trusted is unsound. The direction — key TX waveform meters by slice context, demote the explicit-source map to fallback — is right. The implementation rests on an ordering assumption the client itself does not preserve, and that is where the blockers live.

Scope

Four files, all explained by the issue. Clean checklist; no CHANGELOG; commit signed; tests are socket-free model injection in the already-registered meter_model_test. Preflight: no sockets, no fake peers.

Blockers

1. The slice-context scheme trusts an ordering the client's own decode destroys, the context is never invalidated, and the double-registration defeats both fallbacks (inline ×2). Four facets, one root:

  • Wire order is not preserved. FlexBackend::decodeMeterStatus groups a multi-meter status into a QMap<int, …> and iterates in meter-index order. Flex reuses meter IDs after removal, so a slice created after another was torn down can carry a TX block with a lower reused ID than its own SLC block — processed while m_manifestSliceContext still points at the previous slice, writing bySlice[wrongSlice]. The explicit byTxSource entry holds the right answer, but the new priority order consults the poisoned entry first. The old explicit-first code was immune whenever the source was explicit.
  • The context has no invalidation. It is set on any SLC def and cleared only in clear(), so a mid-session re-announce of a single TX meter (unit change, profile reload) with no preceding SLC block lands under whatever slice defined meters last — overwriting another slice's correct entry.
  • A poisoned entry permanently disables the #4609 fallback. The single-implicit-meter rescue requires bySlice.size() == 1; one stale entry keyed to a dead slice (which removeMeter's by-value purge cannot remove while the meter under it lives) makes it 2 forever — the exact single-modulator dead-gauge class #4609 fixed, reintroduced by bookkeeping.
  • Dual registration defeats the explicit-single-meter case too. A manifest with one explicit-source TX ALC block (SLC context present) fills both maps; move TX to the other slice and bySlice misses, the base-arithmetic key misses, and the fallback is gated on byTxSource.isEmpty() — false — so the gauge dies where the pre-PR scalar followed transmit anywhere. No test covers this shape (the new test covers only the implicit num=0 single-meter case).

Fix directions that keep the PR's (correct) model: iterate the decoded meter group in wire/insertion order and reset m_manifestSliceContext at each message boundary, so context genuinely means "this block's slice"; register byTxSource or bySlice, not both (context-derived defs are slice-known; explicit-context-less defs are source-known); and make the last-resort fallback key on "exactly one distinct meter index across both maps" rather than map emptiness.

2. The clear paths emit 0.0f, which on the dBFS gauge is full-scale top, not "no reading" (inline). kAlcGaugeFloorDbfs is −20; setActiveTxSlice and active-meter removal emit swAlcChanged(0.0f), and the consumer forwards it verbatim whenever the interlock reports transmitting — so a TX-slice reassignment or meter removal mid-key pegs the Phone/CW ALC gauge full red for the transient, and TciServer caches 0.0 as a genuine max-ALC reading for TCI clients. Emit the floor (or an explicit invalid marker the consumer maps to the floor).

Nits (non-blocking)

  • The behavioral narrowing to m_activeTxSlice >= 0 has no in-tree victim — verified: Icom sets txSlice=true before publishing meters, HL2 sets it in its slice delta, and the seam path recomputes on every delta — but the PR's own test edits (setActiveTxSlice(0) added to two previously-passing tests) document the semantic change, and any backend publishing ALC without ever marking a TX slice goes silently dead where it used to work. Allowing the unique-entry fallback when activeTxSlice < 0 costs nothing (one transmitter answers itself) and closes the window.
  • The registration ladder is copy-pasted three times, the resolver ladder three times (this PR itself had to edit all copies in lockstep to flip the priority — the drift risk demonstrating itself), the erase-by-value loop now has six hand-rolled copies where QMap::removeIf is the in-repo idiom, and the SC_* resolvers' deliberate lack of the single-meter fallback is undocumented (the day a single-modulator backend publishes SC_*, #4609 recurs one meter-name over).
  • The ALC branch is now source-narrowed (isTxWaveformMeter && name=="ALC") where the old branch was source-agnostic; no in-tree victim, but an ALC def under any other source now falls through the whole chain with no warning while its values are stored — a qCWarning on an unrouted ALC def would match the project's recent loud-drop commits.
  • The learnings doc's resolution table omits the third rung (single-implicit fallback) and no longer explains that the byTxSource key still comes from the base arithmetic the same doc declares unsound; the code comment stating the 8400M shape omits the firmware version (AGENTS.md wants it at the comment, not only in the doc); convertAlcToGaugeDbfs no longer reads member state and can be file-static; the commit cites Principle XI where the load-bearing principle is I.

What was verified vs read

  • Verified: the FlexLib attachment rule (two passes, Radio.cs line-cited); the context set/clear lifecycle (set on any SLC, cleared only in clear()); the decode-order fact underlying blocker 1a; the fallback gates (activeTxSlice >= 0, byTxSource.isEmpty(), size()==1); the −20 floor vs the 0.0 emits and the interlock-gated consumer; the in-tree no-victim status of the narrowing (Icom/HL2 marking traced to their publish sites); and the test edits documenting the tightening.
  • Refuted along the way: per-packet resolution cost (pre-resolved, clean), stale-map aliasing on index reuse (removeMeter purges both maps correctly), emit-spam on setActiveTxSlice (early-returns), and unit-at-update-time hazards (guarded, equivalent).
  • Not run: no hardware; the 8400M capture is the author's, and the blocker scenarios are constructed from the client's own decode semantics rather than firmware behavior — they need no radio to demonstrate.

Comment thread src/models/MeterModel.cpp Outdated
Comment thread src/models/MeterModel.cpp
@rfoust rfoust self-assigned this Sep 6, 2026
@rfoust
rfoust dismissed ten9876’s stale review September 6, 2026 03:55

The operator authorized fixing the findings and completing approval/squash merge. Both reproduced blockers on ffa9cc7 are addressed in 98872c3: stable exclusive ownership plus wire-order decoding, and -20 dBFS ALC clear values. Regression and mutation checks pass after restoration; thread replies document evidence and why explicit-singleton and per-message-reset suggestions were not adopted wholesale. Dismissing this obsolete blocking review after resolving its threads.

@rfoust rfoust left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The revised PR addresses the ALC-metering portion of #5340. The independent FT8 decode-count concern remains open; the description no longer promises to close the whole issue. Approval is explicitly authorized by the operator after repairing the review findings.

Scope

File/group Purpose Verdict
MeterModel.cpp/.h Active-slice ALC/COMPPEAK/filter routing, stable identity ownership, exclusive registration, selected-unit conversion and empty ALC state In scope
FlexBackend.cpp Preserve the wire ordering that the manifest association depends on In scope
meter_model_test, aetherd_meter_decode_test, phone_cw_level_meter_state_test Socket-free lifecycle, decoder and real gauge regression coverage In scope
flex-meter-learnings.md Captured firmware behavior, compatibility fallbacks and normalized reset semantics In scope

Everything in the diff is explained by the meter-routing fix. No new public protocol or settings surface, UI redesign, CI gate expansion or CHANGELOG change.

Findings

No remaining blockers or nits from this review. Both original failures were reproduced before repair. The thread replies explain the stable ownership and clear-value corrections, plus the evidence-based limits on singleton fallback and cross-message context.

Verification and adversarial checks

  • Reviewed and built 98872c3 including d58e2b8 (the CI base); the contributor history is preserved through a signed merge and signed repair commit.
  • Full ARM64 macOS application build passes with RADE enabled, the prescribed ARM64 toolchain and -j8. Processor and final executable checks confirm ARM64; the RNNoise graph has no x86 sources.
  • All six registered focused tests pass: meter_model_test, aetherd_meter_decode_test, meter_applet_capability_test, meter_applet_voltage_state_test, health_applet_test and phone_cw_level_meter_state_test. These were run directly rather than inferred from filtered PR CI.
  • Tried isolated A re-announcement while B owns TX across ALC, compression and all SC taps; explicit, repeated-zero and mixed 0/9 manifests; nonmonotonic meter IDs; unit changes; removed/repurposed IDs; strict explicit versus implicit singleton fallback; startup/disconnect/slice-change/removal clears. The repaired code survives.
  • Mutation evidence: sorting definitions by ID again fails the decoder/model test, and replacing the presentation-floor clears with zero fails both the model and the actual Phone/CW gauge test. Restored production code was rebuilt and the final six-test run passes.
  • Strict engine-boundary and test-registration checks, generated touchpoint-manifest check and diff whitespace check pass. Test preflight found no new bound/connected sockets or synthetic firmware peers.
  • An isolated offscreen app launch reported QRhi renderer failure and did not expose its automation socket; that process was stopped. The full bridge path and live-radio convergence of this repaired head are not claimed. The production decoder/model and both real Phone/CW gauge widgets provide the behavioral proof. The original author's RF measurements remain clearly labeled as historical, from ffa9cc7. No live radio or TX was used during repair.

Fresh current-main revalidation after CI: upstream/main advanced to 4bf9c5a6 (Icom audio fix #5311). The clean local merge with PR head 98872c35 produced tree 669aabb595fd69430473841941e1098a634b783a. The full ARM64 application build and all six focused CTests pass on that combined tree, as do strict boundary/registration, generated manifest and whitespace checks. The overlap in FlexBackend is separate capability metadata; no repair changes or additional push were needed. The five successful PR CI checks ran with the earlier main d58e2b8a; this newer integration check was performed locally.

All five PR CI checks have passed. Recommendation: squash merge the approved head.

Post-merge verification: squash commit f39e7d27 is signed and verified by GitHub and was confirmed on upstream/main. A concurrent TCI IQ change, 5674c7ab (#4951), became its parent during the final merge window. Its delta leaves the ALC signal consumer and all seven files of this repair intact. The six focused targets were rebuilt and all six tests passed again on the actual merged commit, with strict boundary/registration, manifest and whitespace checks also passing. The full desktop build recorded above was on the preceding 4bf9c5a6 integration tree; this final run rebuilt the focused targets and their production dependencies. Both review threads are resolved and #5340 remains open for its separate FT8 concern.

@rfoust
rfoust merged commit f39e7d2 into aethersdr:main Sep 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants